Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OM] Generalize handling for list creation ops in FreezePaths. #7965

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

mikeurbach
Copy link
Contributor

We previously handled the ListCreateOp specifically, because lists of paths need to become lists of frozen paths. But there are other list creation ops that need to be considered and handled similarly, like the recently added ListConcatOp.

To handle this, the process method for ListCreateOp was updated to work on any generic Operation * that returns a ListType. The typeswitch that dispatches to the process methods was updated to use this generic processor for both ListCreateOp and ListConcatOp. I thought about writing a generic check instead of listing out the supported Operation classes, but that seems like a fragile tradeoff that might not be worth the cost relative to keeping this list up to date.

We previously handled the ListCreateOp specifically, because lists of
paths need to become lists of frozen paths. But there are other list
creation ops that need to be considered and handled similarly, like
the recently added ListConcatOp.

To handle this, the process method for ListCreateOp was updated to
work on any generic Operation * that returns a ListType. The
typeswitch that dispatches to the process methods was updated to use
this generic processor for both ListCreateOp and ListConcatOp. I
thought about writing a generic check instead of listing out the
supported Operation classes, but that seems like a fragile tradeoff
that might not be worth the cost relative to keeping this list up to
date.
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mikeurbach mikeurbach merged commit 116507a into main Dec 10, 2024
4 checks passed
@mikeurbach mikeurbach deleted the mikeurbach/om-list-concat-error-message branch December 10, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants